-
Notifications
You must be signed in to change notification settings - Fork 33
Carry a forker in the mempool #1565
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
11bad3a to
1eabc2b
Compare
506d793 to
bf9c254
Compare
36e01ae to
ae188df
Compare
bf9c254 to
95bd119
Compare
6b00088 to
9d4748d
Compare
0366e24 to
690bfaf
Compare
0972dfc to
c02aea1
Compare
dnadales
reviewed
Jun 23, 2025
ouroboros-consensus/changelog.d/20250620_132627_jasataco_mempool_carry_vh.md
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/API.hs
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Impl/Common.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Impl/Common.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Impl/Common.hs
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Init.hs
Outdated
Show resolved
Hide resolved
dnadales
reviewed
Jun 24, 2025
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Update.hs
Outdated
Show resolved
Hide resolved
dnadales
approved these changes
Jun 24, 2025
5e66a72 to
a7e96d3
Compare
a7e96d3 to
e2d80d1
Compare
c02aea1 to
82392d5
Compare
8676768 to
0e92ed1
Compare
geo2a
approved these changes
Jun 26, 2025
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/API.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Init.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Init.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Update.hs
Outdated
Show resolved
Hide resolved
ouroboros-consensus/src/ouroboros-consensus/Ouroboros/Consensus/Mempool/Init.hs
Show resolved
Hide resolved
|
Note to self: remove now stale mempool traces (TraceMempoolLedgerNotFound for example) |
0e92ed1 to
1940671
Compare
amesgen
reviewed
Oct 28, 2025
Comment on lines
+71
to
+75
| -- With this allocation on the top level registry, we make sure that we first | ||
| -- stop the watcher thread before closing the mempool registry, as otherwise | ||
| -- we would run into a race condition (the thread might try to re-sync and | ||
| -- allocate a forker on the mempool registry which would be closing down). | ||
| void $ allocate topLevelRegistry (\_ -> pure w) cancelThread |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasagredo I came across this code while reviewing #1738. Is this code here still necessary with your recent work in IntersectMBO/io-classes-extra#10?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The mempool will now carry its own forker instead of acquiring one on each
revalidation. This particularly implies that the mempool will no longer
re-sync under the hood while trying to add a transaction, and only the
background thread will perform such a re-sync.
The mempool now has its own registry in which it allocates forkers. The
background thread was moved to this inner registry such that it can access the
mempool internal registry, but an action to cancel it will still live in the
outer registry, to ensure the thread is closed before we attempt to close the
mempool internal registry. Otherwise we would run into a race condition if the
background thread would attempt a resync while the internal registry was being
closed.